align from-less query semantics with SQL #5418
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit changes the interpretation of a query that omits the from operator. Previously, a lake query would use the default "HEAD" parameter as an implied "from HEAD". This contradicts SQL which assumes a table with one empty row (like the implied yield null case). The new assumption in a lake query for a from-less query is an implied null source so that a "select 'hello'" query will return not a single 'hello' rather than a sequence based on the implied read from HEAD. Similary, a from-less command-line query (i.e., super -c) with no file arguments also has an automatic null scan inserted. As part of this change, we moved the logic for inserting the null scan into the DAG (along with the logic for inserting DefaultScan of zio.Readers) into the semantic pass. These changes mean that lake queries that read from a pool require an explicit "from" and there is no impleied pool from HEAD. Because of this, we removed the HEAD references in the code paths that involve lake queries. HEAD is still used by the various "super db" commands to refer to a default pool/branch/commit. Currently, the null scan does not follow the done protocol and restart. We will need to add this when we want from-less subqueries to restart.
nwt
approved these changes
Nov 1, 2024
compiler/parser/api.go
Outdated
| return errors.New("internal error: AST seq cannot be empty") | ||
| return errors.New("delete where command requires an expression") |
There was a problem hiding this comment.
Nit: The parser shouldn't know about the "delete where" command so I'd leave this error string as is.
compiler/parser/api.go
Outdated
| @@ -39,6 +43,9 @@ func ParseQuery(query string, filenames ...string) (*AST, error) { | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| if files.Text == "" { | |||
| return &AST{seq: []ast.Op{}, files: files}, nil | |||
There was a problem hiding this comment.
Nit:
Suggested change
| return &AST{seq: []ast.Op{}, files: files}, nil | |
| return &AST{files: files}, nil |
compiler/semantic/analyzer.go
Outdated
| } else if extInput { | ||
| seq.Prepend(&dag.DefaultScan{Kind: "DefaultScan"}) | ||
| } else { | ||
| // This is a local query and there's not external input |
There was a problem hiding this comment.
Suggested change
| // This is a local query and there's not external input | |
| // This is a local query and there's no external input |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit changes the interpretation of a query that omits the from operator. Previously, a lake query would use the default "HEAD" parameter as an implied "from HEAD". This contradicts SQL which assumes a table with one empty row (like the implied yield null case).
The new assumption in a lake query for a from-less query is an implied null source so that a "select 'hello'" query will return a single 'hello' rather than a sequence based on the implied read from HEAD.
Similary, a from-less command-line query (i.e., super -c) with no file arguments also has an automatic null scan inserted.
As part of this change, we moved the logic for inserting the null scan (along with the logic for inserting DefaultScan of zio.Readers) into the semantic pass (instead of by lake/file compiler).
These changes mean that lake queries that read from a pool require an explicit "from" and there is no implied pool from HEAD. Because of this, we removed the HEAD references in the code paths that involve lake queries. HEAD is still used by the various "super db" commands to refer to a default pool/branch/commit.
Currently, the null scan does not follow the done protocol and restart. We will need to add this when we want from-less subqueries to restart.